Repository navigation
change database and add users table schema - #1
Conversation
📝 WalkthroughWalkthroughThis PR moves the project to PostgreSQL, adds the users schema and queries, updates JWT validation to use the configured secret, and refreshes docs, config examples, ignore rules, and build files. ChangesPostgreSQL migration
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
5abfafd to
080049d
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker-compose.yml`:
- Around line 3-5: The Postgres service is mounting db_data to the wrong data
directory, so the named volume is not being used by postgres:18.4-alpine3.24.
Update the volume mapping in the docker-compose service to target Postgres 18’s
actual data path under /var/lib/postgresql/18/docker, using the existing db_data
volume entry so the container persists state correctly.
In `@README.md`:
- Line 2: The TECH STACK entry in the README still lists MariaDB, which
conflicts with the PostgreSQL migration in this PR. Update the README’s database
value to PostgreSQL so the documented stack matches the actual implementation
and keep the change limited to the existing TECH STACK entry.
In `@sql/schema/001_users.sql`:
- Line 7: The users table definition currently leaves r_id nullable and
unconstrained, so duplicate external IDs can slip in and SelectUserByRID may
return an arbitrary row. Update the schema in the users table DDL to make r_id
NOT NULL and add a UNIQUE constraint (or unique index) on r_id, keeping the
change localized to the column definition used by the user lookup flow.
- Around line 2-9: The users table definition uses a plain integer primary key
for users.id, which requires callers to supply values manually. Update the users
schema so the id column uses an identity-generated primary key in the create
table users statement, keeping the rest of the columns unchanged and ensuring
inserts can omit id safely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a1ea9a5-9c13-4873-a260-44c5a0ba9884
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
.gitignoreREADME.mddocker-compose.ymlgo.modinternal/auth/jwt.gomakefilesql/queries/users.sqlsql/schema/001_users.sqlsqlc.yaml
💤 Files with no reviewable changes (1)
- makefile
080049d to
4e15193
Compare
…te docker compose file for postgres image
4e15193 to
5e06d6c
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/auth/jwt.go (1)
45-50: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winPass
jwt.RegisteredClaimsby pointer toParseWithClaims.
ParseWithClaimsneeds writable claim storage; the current value form makes valid tokens fail JSON decoding. Use&jwt.RegisteredClaims{}(or a namedclaimspointer) and read the subject from that instance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/auth/jwt.go` around lines 45 - 50, `ParseWithClaims` is being called with `jwt.RegisteredClaims` by value in `jwt.ParseWithClaims`, which prevents the claims from being populated correctly. Update the token parsing in the auth JWT flow to pass a pointer to `jwt.RegisteredClaims` (for example via a named claims variable) and then read the subject from that parsed claims instance instead of the zero-value copy.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@internal/auth/jwt.go`:
- Around line 45-50: `ParseWithClaims` is being called with
`jwt.RegisteredClaims` by value in `jwt.ParseWithClaims`, which prevents the
claims from being populated correctly. Update the token parsing in the auth JWT
flow to pass a pointer to `jwt.RegisteredClaims` (for example via a named claims
variable) and then read the subject from that parsed claims instance instead of
the zero-value copy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1ee9ad53-28cf-41d7-a573-a2a53e387cf7
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (11)
.gitignoreREADME.mdconfig/config.tomlconfig/config.toml.exampledocker-compose.ymlgo.modinternal/auth/jwt.gomakefilesql/queries/users.sqlsql/schema/001_users.sqlsqlc.yaml
💤 Files with no reviewable changes (2)
- makefile
- config/config.toml
✅ Files skipped from review due to trivial changes (4)
- README.md
- config/config.toml.example
- .gitignore
- go.mod
🚧 Files skipped from review as they are similar to previous changes (4)
- sqlc.yaml
- sql/queries/users.sql
- sql/schema/001_users.sql
- docker-compose.yml
drop sqlite for postgres because it has more features for uuid and create docker compose file for postgres image
Summary by CodeRabbit